[Flytekit-7005] Adding ray autoscaler options#3413
Conversation
0yukali0
changed the title
0yukali0
changed the title
0yukali0
requested review from
cosmicBboy,
davidmirror-ops,
kumare3,
machichima,
pingsutw,
samhita-alla and
wild-endeavor
as code owners
0yukali0
force-pushed
the
flyte-7005
branch
2 times, most recently
from
d7abd43 to
3fb001d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3413 +/- ##
==========================================
- Coverage 84.05% 83.58% -0.47%
==========================================
Files 327 3 -324
Lines 27885 195 -27690
Branches 3016 0 -3016
==========================================
- Hits 23438 163 -23275
+ Misses 3567 32 -3535
+ Partials 880 0 -880 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Yuteng Chen <[email protected]>
Signed-off-by: Yuteng Chen <[email protected]>
Signed-off-by: Yuteng Chen <[email protected]>
Signed-off-by: Yuteng Chen <[email protected]>
Signed-off-by: Yuteng Chen <[email protected]>
Signed-off-by: Yuteng Chen <[email protected]>
Signed-off-by: Yuteng Chen <[email protected]>
Signed-off-by: Yuteng Chen <[email protected]>
Signed-off-by: Yuteng Chen <[email protected]>
Signed-off-by: Yuteng Chen <[email protected]>
Signed-off-by: Yuteng Chen <[email protected]>
Signed-off-by: Yuteng Chen <[email protected]>
Signed-off-by: Yuteng Chen <[email protected]>
Signed-off-by: Yuteng Chen <[email protected]>
| self.compiled = False | ||
|
|
||
| @property | ||
| def name(self) -> str: # type: ignore[override] |
There was a problem hiding this comment.
Why do we need type: ignore here?
There was a problem hiding this comment.
hi @machichima
I added this when linting indicates the problems.
Currently, should i update code to follow the lint rules in flytekit?
| upscaling_mode=_ray_pb2.AutoscalerOptions.UpscalingMode.Value( | ||
| "UPSCALING_MODE_" + self.upscaling_mode.upper() | ||
| ) |
There was a problem hiding this comment.
Could we follow how we did for Resources enum without transforming to string. User can directly use enum value (e.g. AutoscalerOptions.UpscalingMode.DEFAULT)
flytekit/flytekit/models/task.py
Lines 24 to 30 in 7e289a2
Same comment for from_flyte_idl
Signed-off-by: Yuteng Chen <[email protected]>
Signed-off-by: Yuteng Chen <[email protected]>
Signed-off-by: Yuteng Chen <[email protected]>
pingsutw
left a comment
pingsutw
left a comment
There was a problem hiding this comment.
Thanks, LGTM, could you fix the lint errors
|
|
||
| @property | ||
| def partitions(self) -> Optional[typing.Dict[str, Partition]]: | ||
| def partitions(self) -> typing.Dict[str, Partition]: |
There was a problem hiding this comment.
why do we need to change the type here?
| self.compiled = False | ||
|
|
||
| @property | ||
| def name(self) -> str: |
There was a problem hiding this comment.
Do we use it anywhere? why expose it?
3 participants
Tracking issue
Ralated 7005
Why are the changes needed?
Ray plugin in Flytekit does not support ray autoscaler option when enable_autoscaling is settable.
What changes were proposed in this pull request?
How was this patch tested?
Setup process
clone flyte and flytekit (branch flyte-7005 and flytekit-7005)
run the flyte backend in sandbox
2.1 flytectl demo start --dev
2.2 make compile && POD_NAMESPACE=flyte ./flyte start --config flyte-single-binary-local.yaml
Update ray example like Ray example
3.1 uv venv -p 3.11 && source .venv/bin/activate && uv pip install -e ~/flytekit ~/flytekit/plugins/flytekit-ray && uv pip install -e ~/flyte/flyteidl
3.2 "pyflyte run --remote ray_example.py ray_workflow --n 10"
Screenshots
Check all the applicable boxes
Related PRs
Docs link